-
Notifications
You must be signed in to change notification settings - Fork 762
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support expansion in gator verify #3650
feat: support expansion in gator verify #3650
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3650 +/- ##
==========================================
- Coverage 54.49% 47.72% -6.78%
==========================================
Files 134 236 +102
Lines 12329 19846 +7517
==========================================
+ Hits 6719 9471 +2752
- Misses 5116 9485 +4369
- Partials 494 890 +396
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Are we going to add |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couple small nits
b15146f
to
f9e6699
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@David-Jaeyoon-Lee LGTM after gator tests are modified for expansion template, this might help adding those tests David-Jaeyoon-Lee#2.
I think I added similar tests in this pr already. Or am I misunderstanding something? What specifically do we want to modify? Edit: Nevermind I see |
d3892b4
to
8339ab4
Compare
@David-Jaeyoon-Lee you will probably need to update docs as well to let users know how to use this. This is probably the best place to include that information - https://open-policy-agent.github.io/gatekeeper/website/docs/gator#the-gator-verify-subcommand. I am ok with a follow up PR to update the docs. @maxsmythe @ritazh @sozercan wdyt? |
@@ -128,7 +128,7 @@ gator test --filename=manifests-and-policies/ --output=json | |||
`gator verify` organizes tests into three levels: Suites, Tests, and Cases: | |||
|
|||
- A Suite is a file which defines Tests. | |||
- A Test declares a ConstraintTemplate, a Constraint, and Cases to test the | |||
- A Test declares a ConstraintTemplate, a Constraint, an ExpansionTemplate (optional), and Cases to test the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary to add here or should it just be a separate section.
@@ -162,6 +162,8 @@ ConstraintTemplate. It is an error for the Constraint to have a different type | |||
than that defined in the ConstraintTemplate spec.crd.spec.names.kind, or for the | |||
ConstraintTemplate to not compile. | |||
|
|||
A Test can also optionally compile an ExpansionTemplate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
@@ -264,6 +266,25 @@ the `run` flag: | |||
gator verify path/to/suites/... --run "disallowed" | |||
``` | |||
|
|||
### Validating Generated Resources with ExpansionTemplates |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this is too much or if its not enough (i.e. no example or a more complete example).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@David-Jaeyoon-Lee Thanks for the doc changes. LGTM
Signed-off-by: David-Jaeyoon-Lee <[email protected]>
Signed-off-by: David-Jaeyoon-Lee <[email protected]>
Signed-off-by: David-Jaeyoon-Lee <[email protected]>
Signed-off-by: David-Jaeyoon-Lee <[email protected]>
Signed-off-by: David-Jaeyoon-Lee <[email protected]>
…or gator verify Signed-off-by: David-Jaeyoon-Lee <[email protected]>
Signed-off-by: David-Jaeyoon-Lee <[email protected]>
Signed-off-by: David-Jaeyoon-Lee <[email protected]>
6c1464a
to
6ac1628
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thank you to all who were involved in making this happen! |
What this PR does / why we need it:
It allows for expansion in gator verify.
Which issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when the PR gets merged):Fixes #3432
Special notes for your reviewer: